Skip to content

feat(eks): remove all kubernetes_* resources (v5.0.0 Chunk C) - #54

Merged
obezpalko merged 1 commit into
feat/v5-chunk-b-drop-blueprints-addonsfrom
feat/v5-chunk-c-relocate-k8s-resources
Jul 31, 2026
Merged

obezpalko merged 1 commit into
feat/v5-chunk-b-drop-blueprints-addonsfrom
feat/v5-chunk-c-relocate-k8s-resources

Conversation

@obezpalko

@obezpalko obezpalko commented Jul 31, 2026 •

Copy link
Copy Markdown

User description

Chunk C of the k8s-free module (v5.0.0). Stacked on #53 (Chunk B). Removes every in-cluster resource the module created, each to its GitOps owner:

Resource Destination
storage_class.gp3 / .comet_generic comet-infra umbrella (ArgoCD, already adopted on stsaasuat)
namespace.monitoring comet-infra umbrella (ArgoCD)
secret.monitoring External Secrets Operator (owned)
annotations.app/admin_ns_node_selector DROPPED — obsolete under EKS Auto Mode (NodePools schedule)
namespace.redis_insights agentro-role/rbac local module (comet-devops)

Also drops time_sleep.wait_for_alb_webhook and all now-orphaned vars (module + root + pass-throughs). Kept grafana_admin_user/password — they feed comet_secretsmanager (the AWS SM secret ESO reads), not the EKS module.

No kubernetes_/helm_/kubectl_ resource remains. The kubernetes provider is now declared-but-unused → dropped in Chunk D. terraform init+validate pass.

Cutover is non-destructive (terraform state rm the adopted objects before bumping ?ref) — handled in Chunk E, stsaasuat first.

🤖 Generated with Claude Code


Generated description

Below is a concise technical summary of the changes proposed in this PR:
Remove in-cluster Kubernetes ownership from the EKS module by moving storage classes and monitoring bootstrap to ArgoCD/comet-infra and External Secrets Operator. Drop the now-orphaned Terraform inputs, namespace pinning, redis-insights, and ALB webhook settle delay that only supported those resources.

TopicDetails
Cluster bootstrap Move gp3/comet-generic StorageClasses plus monitoring namespace and Grafana secret ownership out of the EKS module and into GitOps/ESO-managed components.
Modified files (4)
  • main.tf
  • modules/comet_eks/main.tf
  • modules/comet_eks/variables.tf
  • variables.tf
Latest Contributors(2)
UserCommitDate
alexb@comet.comfeat(eks): remove all ...July 31, 2026
CRThazeMerge pull request #52...July 29, 2026
Namespace pinning Remove legacy namespace annotation pinning and the redis-insights namespace now owned by the agentro-role/rbac module under EKS Auto Mode.
Modified files (4)
  • main.tf
  • modules/comet_eks/main.tf
  • modules/comet_eks/variables.tf
  • variables.tf
Latest Contributors(2)
UserCommitDate
alexb@comet.comfeat(eks): remove all ...July 31, 2026
CRThazeMerge pull request #52...July 29, 2026
Review this PR on Baz | Customize your next review

…ree in effect (v5.0.0)

Deletes the last in-cluster resources this module created, moving each to its
GitOps owner:
  - storage_class.gp3 / .comet_generic       -> comet-infra umbrella (ArgoCD)
  - namespace.monitoring                      -> comet-infra umbrella (ArgoCD)
  - secret.monitoring                         -> External Secrets Operator (owned)
  - annotations.app/admin_ns_node_selector    -> DROPPED (obsolete under Auto Mode;
                                                 NodePools/NodeClasses schedule now)
  - namespace.redis_insights                  -> agentro-role/rbac local module

Also removes time_sleep.wait_for_alb_webhook (only the storage classes / monitoring
ns depended on it) and every now-orphaned variable at both module and root level:
enable_monitoring_setup, manage_monitoring_secret, monitoring_namespace,
create_comet_generic_storage_class, storage_class_reclaim_policy (+ eks_* root
aliases), enable_namespace_nodegroup_pinning, app_namespace, admin_pinned_namespaces,
enable_redis_insights_ns, and the comet_eks pass-throughs for all of them.

Kept: grafana_admin_user / grafana_admin_password root vars — they feed
comet_secretsmanager (the AWS Secrets Manager secret ESO reads), not the EKS module.

No kubernetes_/helm_/kubectl_ resource remains in the module. The kubernetes
provider is now declared-but-unused; it is dropped in the next chunk (D) together
with time_sleep.wait_for_cluster_access. terraform init + validate pass.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Comment thread modules/comet_eks/main.tf
Comment on lines +733 to +737
# StorageClasses (gp3 default + comet-generic) moved to the comet-infra umbrella
# chart (ArgoCD-owned) — see comet-devops-helm/charts/comet-infra. The former
# wait_for_alb_webhook settle delay went with them; ordering for ALB-webhook
# dependents is enforced ArgoCD-side (sync waves), not in Terraform. This module
# no longer touches the Kubernetes API for storage classes.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Terraform apply may destroy K8s SCs

modules/comet_eks/main.tf drops ownership of kubernetes_storage_class.gp3 and kubernetes_storage_class.comet_generic without any moved {} or other in-repo state cutover, so upgrades that still have them in Terraform state will plan to destroy them — should we add migration blocks here, and do the same for the monitoring and redis-insights namespace/Secret resources?

Severity

Want Baz to fix this for you? Activate Fixer

Other fix methods

Fix in Cursor

Prompt for AI Agents
Before applying, verify this suggestion against the current code. In
modules/comet_eks/main.tf, add Terraform state migration blocks for the resources
removed from config so upgrades don’t plan destructive changes. Around lines 733-819,
create `moved {}` (preferred) or `removed {}` blocks for kubernetes_storage_class.gp3
and kubernetes_storage_class.comet_generic, matching the correct old resource addresses
exactly as they existed in state. Around lines 1067-1183, do the same for
kubernetes_namespace.monitoring and kubernetes_secret.monitoring. Around lines
1522-1683, do the same for kubernetes_namespace.redis_insights. Follow the same
moved-block style/location already used for EKS addons in this file (lines ~612-635),
and map `to` addresses to where these objects are now managed in-repo
(comet-infra/comet-devops) or use `removed` if they’re now fully external.

Comment thread modules/comet_eks/main.tf
Comment on lines 1522 to +1525
#### Redis Insights namespace ####
#########################################
# Operational debug surface — provides a namespace for the redis-insights helm
# chart (installed by FRED-helm-apply) pinned to the admin NG.

resource "kubernetes_namespace" "redis_insights" {
count = var.enable_redis_insights_ns ? 1 : 0

depends_on = [time_sleep.wait_for_cluster_access]

metadata {
name = "redis-insights"
annotations = {
"scheduler.alpha.kubernetes.io/node-selector" = "nodegroup_name=admin"
}
}
}
# Moved to the agentro-role/rbac local module (comet-devops), which owns agentro's
# in-cluster objects in one place. Not created by this module anymore.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Redis Insights namespace not provisioned

redis-insights namespace creation is dropped from the EKS module and the root enable_redis_insights_ns pass-through, so nothing in-repo now guarantees the namespace exists before operator/port-forward workflows need its RBAC bindings — should we make the agentro-role/rbac replacement create redis-insights on the same enablement paths?

Severity

Want Baz to fix this for you? Activate Fixer

Other fix methods

Fix in Cursor

Prompt for AI Agents
Before applying, verify this suggestion against the current code. In
modules/comet_eks/main.tf around lines 1522-1525 (the “Redis Insights namespace”
section), the code now only contains a comment claiming the redis-insights namespace was
moved and is no longer created here. Fix this by checking the replacement
agentro-role/rbac local module in the comet-devops code (search for redis-insights
namespace/RBAC/port-forward bindings); if it does not create `namespace redis-insights`
(and any required Role/RoleBinding/etc.), add those resources there so the namespace
exists before any workflows rely on it. Also verify how `enable_redis_insights_ns` was
previously used and either (a) re-wire that enablement into the agentro-role/rbac
module, or (b) deliberately make creation unconditional and remove/adjust the old
variable without changing behavior unexpectedly. Finally, ensure there is Terraform
dependency ordering between the provisioning of redis-insights objects and whatever
module/workflow uses them (e.g., via explicit depends_on or module output dependencies).

Comment thread variables.tf
Comment on lines +901 to 903
# StorageClasses moved to the comet-infra umbrella chart (ArgoCD) in v5.0.0.
# eks_storage_class_reclaim_policy / eks_create_comet_generic_storage_class removed.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

StorageClass removals lack migration mapping

The v5.0.0 removal note for StorageClass inputs says eks_storage_class_reclaim_policy / eks_create_comet_generic_storage_class moved to chart values, but it doesn't say which comet-infra umbrella chart values replace them, so downstream configs can't migrate the reclaimPolicy / comet-generic creation contract and v5 input validation will fail — should we document the replacement values?

Severity

Want Baz to fix this for you? Activate Fixer

Other fix methods

Fix in Cursor

Prompt for AI Agents
Before applying, verify this suggestion against the current code. In variables.tf around
lines 901-903, update the “StorageClasses moved to the comet-infra umbrella chart
(ArgoCD) in v5.0.0” comment (the hunk that replaced `eks_storage_class_reclaim_policy`
/ `eks_create_comet_generic_storage_class`) to include an explicit migration mapping for
downstream users. The current note only says the inputs were removed and replaced by
chart values, but it does not name the exact comet-infra umbrella chart value keys that
control (1) StorageClass reclaimPolicy for gp3 and comet-generic and (2) whether
comet-generic StorageClass creation is enabled to avoid Helm/ArgoCD dual ownership.
Refactor by adding the concrete replacement value names and how to set them (including
which behavior is unchanged, e.g., gp3 always created), pulling the exact keys from the
comet-infra umbrella chart’s values.yaml/docs so v5 users can migrate without failing
contract validation.

Comment thread variables.tf
Comment on lines 1628 to +1632
#####################

variable "enable_namespace_nodegroup_pinning" {
description = "Annotate the application namespace with nodegroup_name=comet and admin_pinned_namespaces with nodegroup_name=admin via scheduler.alpha. Skips kube-system + monitoring."
type = bool
default = false
}

variable "app_namespace" {
description = "Application namespace to pin to the comet node group. Defaults to the module environment (which matches the Helm chart's default namespace)."
type = string
default = null
}

variable "admin_pinned_namespaces" {
description = "Namespaces to pin to the admin node group. Defaults cover the cluster's add-on namespaces."
type = list(string)
default = ["cert-manager", "external-dns", "external-secrets"]
}

#####################
#### Redis Insights namespace + agentro port-forward RBAC
#####################

variable "enable_redis_insights_ns" {
description = "Create the redis-insights Kubernetes namespace with scheduler.alpha annotation pinning to admin NG."
type = bool
default = false
}
# scheduler.alpha node-selector pinning (enable_namespace_nodegroup_pinning,
# app_namespace, admin_pinned_namespaces) is obsolete under EKS Auto Mode. The
# redis-insights namespace (enable_redis_insights_ns) moved to the agentro-role/rbac
# local module. All four root vars removed with their comet_eks pass-throughs.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nodegroup/Redis Insights removals lack migration mapping

The v5.0.0 removal note marks the scheduler.alpha annotations obsolete and says redis-insights moved to agentro-role/rbac, but it doesn't say which GitOps-owned inputs/values replace enable_namespace_nodegroup_pinning, app_namespace, admin_pinned_namespaces, and enable_redis_insights_ns or where to set them, so consumers lose the migration path and v5 validation fails if the old args are still present — should we document the replacements?

Severity

Want Baz to fix this for you? Activate Fixer

Other fix methods

Fix in Cursor

Prompt for AI Agents
Before applying, verify this suggestion against the current code. In variables.tf around
lines 1628-1632 under the “Namespace nodegroup pinning + Redis Insights — REMOVED in
v5.0.0” comment block, update the migration note to include a clear replacement
contract for users who previously set enable_namespace_nodegroup_pinning, app_namespace,
admin_pinned_namespaces, and enable_redis_insights_ns. Explicitly document that these
Terraform variables are removed and that consumers must configure the equivalent
behavior via the GitOps/ArgoCD/agentro-role/rbac inputs/values; then search the repo for
where redis-insights namespace creation and any nodegroup pinning behavior are now
controlled (e.g., the agentro-role/rbac module/values files) and reference the exact
values/paths/names in the comment. Also add a short “migration steps” checklist
(what to delete from tfvars, what to set in GitOps) so that users don’t keep passing
removed args and hit v5 input validation failures.

@obezpalko
obezpalko merged commit 3c8a43a into feat/v5-chunk-b-drop-blueprints-addons Jul 31, 2026
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant